feat(tools): add governance hook for memory retrieval (#1348) - #1363
feat(tools): add governance hook for memory retrieval (#1348)#1363AyushKashyapII wants to merge 1 commit into
Conversation
…1348) Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>
|
Great work @AyushKashyapII — this is exactly the hook we need, and the coverage across all integration paths (Vercel, Mastra, VoltAgent, OpenAI, MCP) is thorough. One suggestion for a follow-up (not blocking this PR): @Palo-Alto-AI-Research-Lab raised valid points in #1348 about the return type. The current profile => profile signature works for v1, but a disposition envelope would let callers know why the array got shorter. Something like: interface GovernedRetrievalResult { profile: UserProfile; dispositions?: { memoryId: string; status: 'kept' | 'redacted' | 'blocked'; rule?: string }[]; governanceStatus?: 'success' | 'failed_open' | 'failed_closed'; } This would be backwards-compatible — middlewares that return a plain profile still work; those that return the envelope get richer semantics. Happy to build tealtiger-supermemory on top of this once it lands. The hook interface is clean and covers the right boundary. |
|
@nagasatish007 asked whether the envelope should land before this merges. Read the diff end to end at The envelope, if it lands, has nothing to key on
The API does return ids, they are dropped at the mapping. So either carry the id into There are already two hook contracts here, not one
That is the real argument for deciding the return type now: whatever the envelope ends up being, it has to land on both surfaces in the same commit, or they diverge permanently and every later change is two breaking changes. The VoltAgent search path erases the distinction the scanner needs mostIn results: searchResults.map((r) => ({
memory: r.memory ?? r.chunk ?? "",
...(r.metadata ? { metadata: r.metadata } : {}),
})),Hybrid search returns both memory entries ( That is the one distinction both of you put first in #1348: auto-synced content is the only vector where nobody chose to bring the content in. Everywhere else the hook can make that call; here it structurally cannot. Passing An opt-in feature changes behavior for people who do not opt inSame file, the - searchResults: response.results as Array<{ memory: string; metadata?: ... }>,
+ searchResults: searchResults.map((r) => ({
+ memory: r.memory || r.chunk || "",
+ ...(r.metadata ? { metadata: r.metadata } : {}),
+ })),The old line was a type assertion over the raw SDK results, so Two smaller onesGovernance failure currently reports as a retrieval failure. In Invocation is proven at one of the six call sites. The hook is invoked in The test that catches this is not the redaction one, it is the inverse: a middleware that blocks everything must produce empty, explicitly-blocked output at the caller. A redacting middleware only proves the hook ran on the path it ran on; a blocking one fails loudly the day (Also: the PR is currently conflicting against Disclosure: I am an AI agent (Claude) contributing as part of Palo Alto AI Research Lab. We run a deterministic gate at this same boundary in our own stack, so read the design opinions as coming from someone with a stake in the general problem and none in which SDK anyone picks. |
Summary
Adds an optional governance hook at the memory-retrieval boundary — the point between Supermemory returning
profile()/search()results and those results reaching an agent's context. Closes the generic-hook option raised in #1348.Supermemory doesn't implement PII redaction, prompt-injection detection, or audit logging itself. Instead, this adds a pluggable extension point so any external governance provider (e.g. TealTiger, or an org's own compliance layer) can inspect, rewrite, drop, or block memories before they're formatted and injected into the LLM prompt — without hardcoding a dependency on any one vendor.
Why this approach (vs. the other options in #1348)
The issue proposed three paths: (a) a standalone package needing no core changes, (b) a generic hook API, (c) inline PII/injection detection built into core. This PR implements (b) — it's the only option that's actually a scoped, provider-agnostic change to this repo; (a) needs nothing here, and (c) would mean Supermemory building and maintaining its own redaction/detection logic, which duplicates what governance SDKs already do.
What changed
The hook is a plain function:
(profile, context) => profile | Promise<profile>, called with the raw retrieved memories (not yet deduplicated or formatted) plus{ containerTag, queryText, mode }. It's optional everywhere — omitting it changes nothing.Wired into every place memories get fetched and handed to an LLM:
packages/tools/src/shared/types.ts— newMemoryGovernanceContext/MemoryGovernanceHooktypes, added toSupermemoryBaseOptions.packages/tools/src/shared/memory-client.ts—buildMemoriesText()runs the hook right after the/v4/profilefetch, before dedup/formatting. This is the shared chokepoint used by most integrations.vercel/middleware.ts), Mastra (mastra/processor.ts,mastra/types.ts), VoltAgent (voltagent/middleware.ts) — threaded through tobuildMemoriesText. VoltAgent's separate "advanced search" path (which bypassesbuildMemoriesText) also runs the hook on its raw results.openai/middleware.ts) — has its own duplicated fetch/format logic (doesn't useshared/), so the hook is wired in independently for both the Chat Completions and Responses API paths.apps/mcp/src/client.ts) —SupermemoryClienttakes an optionalgovernance: { onSearch?, onProfile? }hook, applied at the end ofsearch()/getProfile(), right before results become MCP tool output.Not included (open questions for maintainers)
apps/mcp/src/server.ts'sgetClient()doesn't wire a hook in — there's no existing config mechanism to source one from at request time in the hosted Cloudflare Workers deployment, and inventing one (env var, webhook, etc.) felt like a separate architectural decision.Test plan
shared/memory-client.test.ts: stubs/v4/profileto return a memory containing a fake SSN, passes agovernanceHookthat redacts SSN-shaped strings, and asserts both (1) the hook receives the raw profile + correct context, and (2) the final formatted output contains[REDACTED]and never the real SSN.bun run testinpackages/tools— 90 passing, no regressions (2 pre-existing unrelated failures: one needs a liveSUPERMEMORY_API_KEY, one has a pre-existing broken import onmain).bun run check-types— no new errors introduced; confirmed viagit stashthat the one remaining error inopenai/middleware.tspre-dates this change, and this change incidentally fixes a pre-existing type error involtagent/middleware.ts